fix: fully stateless OAuth tokens, drop HOMEASSISTANT_TOKEN requirement - #893
Conversation
…irement Fixes homeassistant-ai#886: OAuth mode no longer requires HOMEASSISTANT_TOKEN env var. When the var is empty/unset, main_oauth() sets the sentinel value so Settings validation passes. Fixes homeassistant-ai#837: Both access and refresh tokens are now stateless (base64-encoded JSON containing the HA LLAT, type, client_id, scopes, and expiry). No server-side token state is stored, eliminating oauth_state.json and all disk I/O. Tokens survive container restarts by design — clients re-register via DCR automatically. Removed: _save_state(), _load_state(), _refresh_to_access_map, state_dir parameter, get_ha_credentials_for_token(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the OAuth provider to be fully stateless, significantly improving reliability in containerized environments by eliminating the need for persistent disk storage. By encoding necessary credentials directly into self-contained tokens, the system now handles session management transparently across restarts, while maintaining the existing security model and improving overall maintainability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the OAuth provider to a fully stateless architecture, eliminating the need for server-side state persistence and allowing the server to survive restarts without losing sessions. Both access and refresh tokens now carry the Home Assistant Long-Lived Access Token (LLAT) and necessary metadata as base64-encoded JSON. The changes include the removal of disk-based state management, the introduction of a unified token encoding/decoding mechanism, and significant updates to the test suite to reflect the new stateless behavior. Feedback is provided regarding a potential KeyError when recovering tokens and the need for more robust JSON validation during token decoding.
json.loads can return non-dict types (list, str, int) for valid JSON. Without isinstance check, calling .get() on a non-dict payload would raise AttributeError, which is not in the except clause. Addresses Gemini Code Assist review feedback on PR homeassistant-ai#893. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sergeykad
left a comment
sergeykad
left a comment
There was a problem hiding this comment.
9. to_dict docstring says "for storage" but storage is removed (provider.py:55)
This method's docstring reads """Convert to dictionary for storage.""" but the PR removes all disk persistence. The method appears unused outside test_credentials_to_dict. Either remove it or update the docstring to """Convert to dictionary representation.""". Also: the validated_at field (line 53) is set but never read anywhere in the codebase — consider removing it.
1. HMAC-sign refresh tokens to prevent tampering — payload is signed with a per-instance server secret, verified on decode. Raw LLAT remains in the signed payload (needed for token exchange) but the signature prevents modification of any field. 2. Enforce exp on access tokens — encode exp in access tokens and reject expired ones in load_access_token. 3. Add logging to revoke_token with RFC 7009 note. 4. Add tests for main_oauth OAUTH_MODE_TOKEN sentinel logic. 5. Add warning logs for security-sensitive token rejections (refresh-as-access, client_id mismatch). 6. Use .pop() instead of bare del for ha_credentials cleanup. 7. Update get_ha_credentials docstring for new architecture. 8. Add tests for _decode_token with non-dict JSON payloads. 10. Update _encode_token docstring to describe signing and exp. 11. Add version anchor to backwards-compat comment (v7.x / April 2026). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses sergeykad's follow-up tying items 1-3 together: without signing access tokens, an attacker who intercepts one can extract the LLAT and forge arbitrary tokens with any claims. Now both access and refresh tokens are HMAC-signed using a per-instance server secret. _decode_token verifies signatures on all tokens, with backwards-compat fallback for unsigned pre-signing tokens. Tokens no longer survive provider restart (new HMAC secret each startup). Updated tests accordingly: test_tokens_survive_provider_restart → test_tokens_invalidated_on_provider_restart, and test_chained_refresh_across_restart → test_chained_refresh_same_instance. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Storage was removed in this PR, so to_dict() and validated_at are dead code. Removed both and the test_credentials_to_dict test. Updated class docstring to reflect transient-only purpose. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…anther13/ha-mcp-fork into fix/oauth-stateless-tokens
PR homeassistant-ai#908 added image caching to e2e-tests.yml and pr.yml but missed performance-tests.yml. Without caching, the workflow hits GHCR rate limits on every run, causing all performance tests to fail at setup. Adds the same HA_IMAGE_GHCR env var, actions/cache, and GHCR→Docker Hub fallback pattern used by the other workflows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The HMAC secret is regenerated on every restart, so all pre-existing tokens are already invalidated. The only scenario for an unsigned token reaching a running server is an attacker crafting a plain base64 blob to bypass HMAC verification. Remove the path entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
CI fix: Docker image caching added to Performance tests were failing on all PRs with Fixed in 5e2566f — added |
Unsigned tokens are now rejected (commit 5e8f4db). Updated test from asserting acceptance to asserting rejection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🧪 Your changes are now in the dev channel!Your PR has been merged to master and is available for testing in the dev channel. Test your changes before the next stable release (biweekly Wednesday): Quick start# Run dev version
uvx ha-mcp-dev
# Check version
uvx ha-mcp-dev --versionDocker: docker pull ghcr.io/homeassistant-ai/ha-mcp:dev
docker run --rm -i \
-e HOMEASSISTANT_URL=http://your-ha:8123 \
-e HOMEASSISTANT_TOKEN=your_token \
ghcr.io/homeassistant-ai/ha-mcp:devFound an issue? Please open a new bug report and mention this PR for context. |
2 participants
What does this PR do?
Fixes #886 and #837 by making the OAuth provider fully stateless — both access and refresh tokens are now self-contained base64 JSON, eliminating all server-side token storage and disk persistence.
#886 — OAuth still requires HOMEASSISTANT_TOKEN:
main_oauth()now sets theOAUTH_MODE_TOKENsentinel whenHOMEASSISTANT_TOKENis empty/unset, soSettingsvalidation passes without a server-level token. This is a one-line fix in the entrypoint.#837 — OAuth state lost on container restart: Refresh tokens are now stateless (encoded with
ha_token,client_id,scopes,exp, andtype), matching the existing stateless access token pattern. This removes_save_state(),_load_state(),_refresh_to_access_map, thestate_dirparameter, andoauth_state.jsonentirely. Tokens survive container restarts by design — clients re-register via DCR automatically and transparently.Security model unchanged: The LLAT is the authorization boundary. Revoking it in Home Assistant immediately invalidates all derived tokens. Server-side revocation was previously a no-op in practice (in-memory state lost on restart anyway).
Backwards compatibility:
load_access_token()accepts tokens withtype=accessor notypefield (old format). Refresh tokens from old format (random strings stored server-side) won't survive a restart — which was already the bug being fixed.Net result: −226 lines, zero disk I/O, container-restart-safe.
Type of change
Testing
uv run pytest)uv run ruff check)Checklist